-
-
Notifications
You must be signed in to change notification settings - Fork 421
Fixes operationId starting with integers crashes generation #1326
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 41a0b20 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Hello @thejhh ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes issue #952 by ensuring operationIds starting with numbers are quoted in generated TypeScript to avoid invalid identifiers.
- Adds a test schema and snapshot for operationIds beginning with digits
- Updates templates to wrap invalid identifiers in quotes
- Documents the change in a new changeset
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/spec/operationId-starting-with-number/schema.json | Added OpenAPI schema with operationIds beginning with numbers |
| tests/spec/operationId-starting-with-number/basic.test.ts | New test verifying quoted vs. unquoted operationId names |
| tests/spec/operationId-starting-with-number/snapshots | Snapshot updated to include quoted identifiers |
| templates/modular/procedure-call.ejs | Wrapped invalid procedure call names in quotes |
| templates/default/procedure-call.ejs | Wrapped invalid procedure call names in quotes |
| templates/base/route-type.ejs | Wrapped invalid namespace names in quotes |
| .changeset/fix-operationid-starting-with-number.md | Added release notes for this patch |
Comments suppressed due to low confidence (1)
| } | ||
| } | ||
| const isValidIdentifier = (name) => /^[A-Za-z_$][A-Za-z0-9_$]*$/.test(name); |
Copilot
AI
Jul 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The helper function isValidIdentifier is duplicated across multiple templates; consider extracting it into the shared utils object or a common helper file to centralize this logic and simplify future updates.
| const { query, payload, pathParams, headers } = route.request; | ||
| const routeDocs = includeFile("@base/route-docs", { config, route, utils }); | ||
| const isValidIdentifier = (name) => /^[A-Za-z_$][A-Za-z0-9_$]*$/.test(name); |
Copilot
AI
Jul 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extracting isValidIdentifier into a shared utility would avoid repeating the same regex in multiple template files and ensure consistency if validation rules change.
The modular template was incorrectly using object method syntax (`:` and `,`) when route.namespace was present, introduced by PR acacode#1326. This caused TypeScript syntax errors in generated code. The modular template should always generate class properties with arrow functions (`=` and `;`), regardless of namespace presence. This resolves the issue reported in acacode#1366 where version 13.2.8 generated invalid TypeScript code with modular templates. Signed-off-by: Sora Morimoto <[email protected]>
The modular template was incorrectly using object method syntax (`:` and `,`) when route.namespace was present, introduced by PR acacode#1326. This caused TypeScript syntax errors in generated code. The modular template should always generate class properties with arrow functions (`=` and `;`), regardless of namespace presence. This resolves the issue reported in acacode#1366 where version 13.2.8 generated invalid TypeScript code with modular templates. Signed-off-by: Sora Morimoto <[email protected]>
* Add test that generates content for modular (based on 13.2.7) * format * Fix modular template to always generate class property syntax The modular template was incorrectly using object method syntax (`:` and `,`) when route.namespace was present, introduced by PR #1326. This caused TypeScript syntax errors in generated code. The modular template should always generate class properties with arrow functions (`=` and `;`), regardless of namespace presence. This resolves the issue reported in #1366 where version 13.2.8 generated invalid TypeScript code with modular templates. Signed-off-by: Sora Morimoto <[email protected]> --------- Signed-off-by: Sora Morimoto <[email protected]> Co-authored-by: Sora Morimoto <[email protected]>
Fixes issue #952 by quoting invalid operationIds using double quotes.